Ccip-4308 Test logic optimizations for docker node tests #15392
Conversation
AnieeG
requested review from
winder,
rstout,
asoliman92,
makramkd,
dimkouv,
mateusz-sekara and
0xnogo
| e.Logger.Info("ccip multicall already deployed", "addr", mc3.Address) | ||
| if mc3 != nil { | ||
| e.Logger.Info("ccip multicall already deployed", "addr", mc3.Address) | ||
| } |
| var srcToken *burn_mint_erc677.BurnMintERC677 | ||
| var srcPool *burn_mint_token_pool.BurnMintTokenPool | ||
| var dstToken *burn_mint_erc677.BurnMintERC677 | ||
| var dstPool *burn_mint_token_pool.BurnMintTokenPool |
There was a problem hiding this comment.
nit/ can combine into one var decl
| @@ -130,7 +134,20 @@ func NewLocalDevEnvironment( | |||
|
|
|||
| // fund the nodes | |||
| zeroLogLggr := logging.GetTestLogger(t) | |||
| FundNodes(t, zeroLogLggr, testEnv, cfg, don.PluginNodes()) | |||
| // check if we can fund in parallel | |||
There was a problem hiding this comment.
When wouldn't we want to fund in parallel? Real testnets?
| @@ -437,6 +455,12 @@ func CreateDockerEnv(t *testing.T) ( | |||
| evmNetworks[i].URLs = rpcProvider.PrivateWsUrsl() | |||
| } | |||
| env.EVMNetworks = append(env.EVMNetworks, &evmNetworks[i]) | |||
| // if number of private keys is more than 1, we can fund the nodes in parallel | |||
There was a problem hiding this comment.
Oh I see, is this logic really required? Why not just always fund in parallel? Can't imagine it'd add that much overhead
There was a problem hiding this comment.
Do you mean change it to having >2 keys in private keys always and fail otherwise?
| @@ -173,28 +178,41 @@ func TestUSDCTokenTransfer(t *testing.T) { | |||
| }, | |||
| } | |||
|
|
|||
| for i, tt := range tcs { | |||
| tcs[i].initialBalances = make(map[common.Address]*big.Int) | |||
There was a problem hiding this comment.
I think this doesn't seem right. Every test can pick any token / any chain to transfer. Therefore any sequential test relies on the outcome of the previous one. For instance, if test1 sends a token from A -> B, and test2 does a similar work, you will get the wrong initial balance for test2, because in the meantime that balance will be changed by test1 execution.
If it doesn't fails then probably it's mainly because we pick different combinations for every test case. However, adding new test case will probably break it, no?
There was a problem hiding this comment.
Every test is using a different receiver right? will that not make it separate from each other
Requires
Supports